Bucket level multi-tenant trigger#2098
Merged
eirsep merged 3 commits intoopensearch-project:mainfrom Apr 29, 2026
Merged
Conversation
eirsep
requested review from
AWSHurneyt,
amsiglan,
bowenlan-amzn,
engechas,
getsaurabh02,
goyamegh,
jowg-amazon,
lezzago,
praveensameneni,
rishabhmaurya,
riysaxen-amzn,
sbcd90 and
toepkerd
as code owners
eirsep
force-pushed
the
bucket-level-multi-tenant-trigger
branch
3 times, most recently
from
6f13e7d to
d82f8ad
Compare
eirsep
requested review from
KashKondaka,
manaswini1920,
nishtham-amazon and
vikhy-aws
as code owners
engechas
reviewed
Apr 20, 2026
| * Extracts include/exclude regex patterns from [IncludeExclude]. | ||
| * Uses reflection to access private fields since no public API exposes the raw patterns. | ||
| */ | ||
| private fun extractPatterns(includeExclude: IncludeExclude): Pair<Pattern?, Pattern?> { |
Collaborator
There was a problem hiding this comment.
Reflection is an anti-pattern and won't be supported in some environments. Is there a way to take a dependency on the IncludeExclude object or recreate it in a way that we can access the fields without reflection?
Member
Author
There was a problem hiding this comment.
changed to parse the json
| } | ||
| } | ||
| if (found == null) { | ||
| throw IllegalArgumentException("ParentBucketPath: $parentBucketPath not found in query aggregations") |
Collaborator
There was a problem hiding this comment.
Throwing the exception here would only allow one iteration of the for loop right? I'm wondering if that's the intended behavior or if we are looking for the parentAgg in any of the available pathElements
Member
Author
There was a problem hiding this comment.
There are two nested loops:
- Outer loop: iterates over pathElements (e.g., for agg1>agg2, it walks agg1 then agg2)
- Inner loop: searches the current level's agg builders for the matching name
The throw is inside the outer loop but outside the inner loop. If agg1 is found, the outer loop continues to look for agg2 inside agg1's sub-aggregations. The exception only fires if a path element isn't found at its expected level — which is correct behavior imo.
| * since the field is private with no public getter in the commons library. | ||
| */ | ||
| @Suppress("UNCHECKED_CAST") | ||
| private fun extractBucketsPathsMap(selector: BucketSelectorExtAggregationBuilder): Map<String, String> { |
Collaborator
There was a problem hiding this comment.
Same comment on reflection
| } | ||
| } | ||
|
|
||
| private fun isIndexNotFoundException(e: Throwable): Boolean { |
Collaborator
There was a problem hiding this comment.
Was this from another PR? It looks familiar and doesn't appear to be used anywhere
…ard bucket_selector Replace the custom BucketSelectorExt aggregation with standard bucket_selector for bucket-level trigger evaluation when multi_tenant_trigger_eval_enabled is true. This enables trigger evaluation on user clusters that do not have the alerting plugin installed. When the flag is on, bucket-level monitors are limited to 1 trigger. The standard bucket_selector is injected directly into the monitor's search query so a single search call performs both data collection and trigger evaluation. Include/exclude filtering from BucketSelectorExtFilter is replicated post-response by BucketKeyFilter. The existing BucketSelectorExt path is completely unchanged when the flag is off (the default). New files: - BucketSelectorQueryBuilder: injects standard bucket_selector sub-agg - BucketKeyFilter: post-response include/exclude bucket filtering - TriggerService.runBucketLevelTriggerFromFilteredResponse(): reads remaining buckets from filtered response Modified files: - BucketLevelMonitorRunner: flag-gated single-query path - InputService: useStandardBucketSelector parameter - TransportIndexMonitorAction: 1-trigger validation for bucket-level monitors when flag is on - AggregationQueryRewriter: skipBucketSelectorInjection parameter Integration tests: - RemoteBucketLevelTriggerIT: 13 tests covering composite/terms agg, alert lifecycle, dry run, nested parent path, include/exclude filter, validation of 1-trigger limit, Painless script in query - RemoteBucketLevelTriggerRegressionIT: 5 tests verifying flag=false preserves existing BucketSelectorExt behavior Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
- Remove collectInputResultsForTrigger() from InputService (dead code from per-trigger query approach) - Remove skipAllBucketSelectorInjection parameter from collectInputResults and getSearchRequest (no longer used) - Collapse identical triggerCtx if/else branches in BucketLevelMonitorRunner - Restore .gitignore to original state Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
…dation - BucketSelectorQueryBuilder: use public bucketsPathsMap getter from common-utils instead of reflection (opensearch-project/common-utils#949) - BucketKeyFilter: use IncludeExclude.convertToStringFilter() instead of reflection on private fields - TriggerService: replace blanket @Suppress(UNCHECKED_CAST) with runtime type checks using require() - BucketLevelMonitorRunner: add 1-trigger validation at execution time to cover the _execute API path (in addition to create/update validation) Signed-off-by: Surya Sashank Nistala <snistala@amazon.com>
eirsep
force-pushed
the
bucket-level-multi-tenant-trigger
branch
from
d82f8ad to
31f8028
Compare
engechas
approved these changes
Apr 29, 2026
AWSHurneyt
approved these changes
Apr 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace the custom BucketSelectorExt aggregation with standard
bucket_selector for bucket-level trigger evaluation when
multi_tenant_trigger_eval_enabled is true. This enables trigger
evaluation on user clusters that do not have the alerting plugin
installed.
When the flag is on, bucket-level monitors are limited to 1 trigger.
The standard bucket_selector is injected directly into the monitor's
search query so a single search call performs both data collection and
trigger evaluation. Include/exclude filtering from BucketSelectorExtFilter
is replicated post-response by BucketKeyFilter.
The existing BucketSelectorExt path is completely unchanged when the
flag is off (the default).